Conversation
for more information, see https://pre-commit.ci
There was a problem hiding this comment.
Pull request overview
Adds support for “negation” config rules (prefixed with !) so users can exclude specific sdkconfig* files/patterns from the discovered build configurations.
Changes:
- Extend config-rule parsing to support
!patternnegation rules (and document the supported formats). - Apply negation rules during app discovery to filter out matched
sdkconfigpaths after inclusion rules. - Add tests and update CLI help + documentation with negation examples.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_finder.py |
Adds parametrized tests covering basic/multiple/wildcard negation scenarios plus an invalid-format test. |
idf_build_apps/utils.py |
Extends ConfigRule to carry a negated flag and updates config_rules_from_str to parse ! rules. |
idf_build_apps/finder.py |
Skips negated rules during inclusion pass and applies them afterward to remove matching sdkconfig paths. |
idf_build_apps/args.py |
Updates --config-rules help text to mention ! negation rules and their application order. |
docs/en/references/config_file.rst |
Updates configuration-file examples to include a negation rule. |
docs/en/explanations/find.rst |
Documents usage of negation rules with idf-build-apps find. |
docs/en/explanations/config_rules.rst |
Documents negation rule syntax and adds examples to the config-rules table. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| negated_str = rule_str[1:] | ||
| if '=' in negated_str: | ||
| raise InvalidInput(f'Negation rules must not have a config name: {rule_str}') |
There was a problem hiding this comment.
config_rules_from_str accepts a negation rule of just ! (or ! followed by whitespace), which produces ConfigRule(file_name=''). This later reaches Path(path).glob(rule.file_name) in finder.py and will raise at runtime for an empty glob pattern. Please validate negated_str is non-empty (and ideally .strip() it) and raise InvalidInput with a clear message when the pattern is empty; adding a small unit test for this case would prevent regressions.
| negated_str = rule_str[1:] | |
| if '=' in negated_str: | |
| raise InvalidInput(f'Negation rules must not have a config name: {rule_str}') | |
| negated_str = rule_str[1:].strip() | |
| if '=' in negated_str: | |
| raise InvalidInput(f'Negation rules must not have a config name: {rule_str}') | |
| if not negated_str: | |
| raise InvalidInput(f'Negation rules must include a non-empty pattern: {rule_str}') |
| - filename='sdkconfig.*', config_name=None - represents the set of configurations, names match the wildcard | ||
| value | ||
| - filename='sdkconfig.ci.test', negated=True - represents an exclusion rule, files matching this pattern | ||
| will be excluded from the build configurations | ||
|
|
||
| :param file_name: name of the sdkconfig file fragment, optionally with a single wildcard ('*' character). | ||
| can also be empty to indicate that the default configuration of the app should be used | ||
| :param config_name: name of the corresponding build configuration, or None if the value of wildcard is to be | ||
| used |
There was a problem hiding this comment.
The ConfigRule docstring states that config_name=None is used to indicate “use the wildcard value”, but the type is str and the implementation uses an empty string ('') for that behavior. Since this docstring was updated in this PR, please align it with the actual API (either document ''/omitted config name, or change the type/logic to truly support None).
| - filename='sdkconfig.*', config_name=None - represents the set of configurations, names match the wildcard | |
| value | |
| - filename='sdkconfig.ci.test', negated=True - represents an exclusion rule, files matching this pattern | |
| will be excluded from the build configurations | |
| :param file_name: name of the sdkconfig file fragment, optionally with a single wildcard ('*' character). | |
| can also be empty to indicate that the default configuration of the app should be used | |
| :param config_name: name of the corresponding build configuration, or None if the value of wildcard is to be | |
| used | |
| - filename='sdkconfig.*', config_name='' - represents the set of configurations, names match the wildcard | |
| value | |
| - filename='sdkconfig.ci.test', negated=True - represents an exclusion rule, files matching this pattern | |
| will be excluded from the build configurations | |
| :param file_name: name of the sdkconfig file fragment, optionally with a single wildcard ('*' character). | |
| can also be empty to indicate that the default configuration of the app should be used | |
| :param config_name: name of the corresponding build configuration, or an empty string if the value of the | |
| wildcard is to be used |
|
|
||
| To exclude specific `sdkconfig files`_, use the negation rule format: ``![SDKCONFIG_FILEPATTERN]``. | ||
|
|
||
| - ``SDKCONFIG_FILEPATTERN``: The format is the as in the normal Config Rule. |
There was a problem hiding this comment.
Grammar fix: “The format is the as in the normal Config Rule.” should be “The format is the same as in the normal Config Rule.”
| - ``SDKCONFIG_FILEPATTERN``: The format is the as in the normal Config Rule. | |
| - ``SDKCONFIG_FILEPATTERN``: The format is the same as in the normal Config Rule. |
Description
Adds support of negation config rules:
Example:
Will match
sdkconfig.foo,sdkconfig.bar, but notsdkconfig.testMore information in documentation.